Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC: deprecate hex2num and num2hex, alternative to #22088 #22203

Closed
wants to merge 5 commits into from

Conversation

rfourquet
Copy link
Member

This is point 2 in #22088 (comment).

  • num2hex(x) is deprecated in favor of bits(x, hex), as bits is also a function to give the string representation of the bit pattern of a bitstype value.
  • hex2num(str) is deprecated in favor of reinterpret(T::Type, str): this one may be controversial, as it may also mean reinterpret the bytes in the string as a value of type T. For this last operation, this could also be done more explicitly by reinterpret(T, Vector{UInt8}(str)).
    An alternative to reinterpret would be to use parse with e.g. a binary=true keyword.

This is based off #22039, which was fixing bugs in those functions (this present PR introduces only one new commit).

@dpsanders
Copy link
Contributor

Isn't Hex2num just parse(s, 16)?

@mbauman
Copy link
Member

mbauman commented Jun 3, 2017

Not quite — it's reinterpret(Float64, parse(UInt64, s, 16)).

I think a hypothetical parse(Float64, s, 16) would interpret a string without decimals like Float64(parse(Int64, s, 16)).

@rfourquet rfourquet force-pushed the rf/hex2num-deprecate branch from c5bc5b4 to ef3e42e Compare June 3, 2017 19:12
@rfourquet rfourquet force-pushed the rf/hex2num-deprecate branch from ef3e42e to d7771bf Compare June 3, 2017 19:15
@tkelman
Copy link
Contributor

tkelman commented Jun 3, 2017

IMO reinterpret on a string is not the right function to use for this.

@rfourquet
Copy link
Member Author

rfourquet commented Jun 4, 2017

I'm also not convinced about reinterpret on a string, although it depends on the definition we give to reinterpret! I feel parse is also not the right function. Why not a new frombits function (does the inverse of bits) ?

@Sacha0 Sacha0 added the deprecation This change introduces or involves a deprecation label Jun 4, 2017
@fredrikekre
Copy link
Member

Given #22088, is this still relevant?

@JeffBezanson
Copy link
Member

Let's revive the useful non-num2hex parts of this.

@ViralBShah
Copy link
Member

Do we still need this one - or can we close?

@rfourquet
Copy link
Member Author

As the other straightforward deprecation of num2hex/hex2num was chosen, I'm not sure what is the "useful non-num2hex parts of this" Jeff is refering to. I personally still wish the bitstring(x, hex) as another spelling of num2hex.

@StefanKarpinski
Copy link
Member

bitstring(x, 16) would be a perfectly ok spelling in my view, if you want to make a PR, but I have to say that I'm very strongly in favor of PRs that remove functions since we can always add methods to functions like bitstring in the future. What do you think the general rule would be for bases that don't have the property that the base divides the number of bits in x? Error? Pad with zeros to the length of the longest possible number of digits? I.e. length(bitstring(typemax(x), base)).

@DilumAluthge DilumAluthge deleted the rf/hex2num-deprecate branch March 25, 2021 22:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deprecation This change introduces or involves a deprecation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants